feat/db: cleanup old account state data db entries#1645
Conversation
drahnr
force-pushed
the
miden-node-bernhard-cleanup-old-db-entries-main
branch
from
2cd6161 to
eb4daa5
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements database cleanup functionality for historical account state data to prevent unbounded database growth. The implementation adds a cleanup routine that runs during apply_block to delete old vault asset and storage map entries while preserving the latest state and recent history.
Changes:
- Added
cleanup_all_accountsfunction that deletes historical entries older than 50 blocks (excludingis_latest=trueentries) - Created database migration to add partial indices on
account_vault_assetsandaccount_storage_map_valuestables for efficient cleanup queries - Integrated cleanup into the
apply_blocktransaction in the store component
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/store/src/db/models/queries/accounts.rs | Adds cleanup_all_accounts function with 50-block retention policy for vault assets and storage map values |
| crates/store/src/db/mod.rs | Integrates cleanup call into apply_block transaction after block data is written |
| crates/store/src/db/migrations/2026020600000_cleanup_indices/up.sql | Creates partial indices for efficient cleanup queries on non-latest entries |
| crates/store/src/db/migrations/2026020600000_cleanup_indices/down.sql | Provides rollback script to remove cleanup indices |
| crates/store/src/errors.rs | Adds QueryTimeout error variant (unused in this PR) |
| crates/store/src/inner_forest/mod.rs | Adds TODO comment about tying in-memory forest retention to DB pruning policy |
| crates/store/src/db/tests.rs | Renames test function for consistency |
| CHANGELOG.md | Adds changelog entry (duplicate of existing v0.13.0 entry) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
drahnr
force-pushed
the
miden-node-bernhard-cleanup-old-db-entries-main
branch
from
d333137 to
b78fcb5
Compare
|
|
| }; | ||
| use loader::{load_mmr, load_smt_forest, verify_tree_consistency}; | ||
|
|
||
| mod apply_block; |
There was a problem hiding this comment.
This makes it harder to review, but if we want further changes to apply_block - which we do, we should keep the sync on the extra file with next to avoid more pain when merging.
drahnr
force-pushed
the
miden-node-bernhard-cleanup-old-db-entries-main
branch
from
32d068d to
3506452
Compare
| /// | ||
| /// # Returns | ||
| /// A tuple of `(vault_assets_deleted, storage_map_values_deleted)` | ||
| pub fn cleanup_all_accounts( |
There was a problem hiding this comment.
Organizationally, should we not have:
fn prune_history(..) {
let cutoff_block = i64::from(chain_tip.as_u32().saturating_sub(HISTORICAL_BLOCK_RETENTION));
prune_account_vault_assets()?;
prune_account_storage_maps()?;
// Probably more that we will need.
}
fn prune_account_vault_assets(...) {}
fn prune_account_storage_map(...) {}and ideally we should also instrument each of these so we know how long each took.
2 participants
Important
Targeting
mainWe recently started tracking historical account state storage ( #1227 ) but that punted on the cleanup, causing unbounded growth of the DB.
The PR adds a cleanup routine to
apply_block.Pro:
Con:
apply_block